Update to `react-router@6` #979

leventebalogh · 2024-11-26T15:25:31Z

Fixes #608

Updates the scenes packages to use react-router v6.
This probably requires a major version bump, as it's going to be a breaking change for plugins that depend on scenes.

Why?

We are in the process of migrating Grafana core and also internal plugins to use react-router v6, however any plugin that is depending on scenes cannot easily do it (or at least it's a complicated task). Scenes still using react-router v5 is a blocker for plugins that would like to update.

Huge thanks to @jackw for the help of discovering and removing some of the circular dependences in the scenes package that were causing Jest to go mental.

Release notes

This release marks a big change where scene app based routing is using react-router v6 under the hood. This sadly requires several changes to plugins as react-router now requires all route paths to be relative.

Updating a plugin

Example plugin update PR →

Bump @grafana/scenes
Bump react-router-dom

-"react-router-dom": "^5.2.0",
+"react-router-dom": "^6.22.0",

Remove @types/react-router-dom

-"@types/react-router-dom": "^5.2.0",

<Route>: use relative wildcard routes

-<Route path="/a/myorg-example-app/home ... />
+<Route path="home/*" ... />

<Route>: use the element prop

-<Route ... component={WithDrilldown} />
+<Route ... element={<WithDrilldown />} />

<Switch>: replace with Routes

-import { Switch } from "react-router-dom";
+import { Routes } from "react-router-dom";  

-<Switch> ... </Switch>
+<Routes> ... </Routes>

Add a relative routePath prop to each <SceneAppPage>. It needs to include a wildcard if there are any drill-down or tab pages under it.

-new SceneAppPage({
-  url: "/a/myorg-example-app/home",
-  ...
- });
+new SceneAppPage({
+  url: "/a/myorg-example-app/home",
+  routePath: "home/*",
+  ...
+})

** Make the routePath prop defined on drill-downs relative and include a wildcard. **

Testing

The following tests were done with a locally built Grafana and new plugin scaffolds.

Grafana with old scenes version & Plugin with new scenes version
Grafana with new scenes version & Plugin with old scenes version
Grafana with new scenes version & Plugin with new scenes version

📦 Published PR as canary version: 6.0.0--canary.979.12746556745.0

✨ Test out this PR locally via:

npm install @grafana/[email protected]
npm install @grafana/[email protected]
# or 
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]

torkelo · 2024-11-27T09:03:17Z

Pushed an update to get SceneAppPage and most demos to work

Scene apps needs a big update

add routePath to all SceneAppPage definitions (with * wildcard if they have drilldowns) this needs to be relative not absolute
the url property on SceneAppPage needs to remain and be absolute as it's used to create breadcrumbs and tab links
routePath on all drilldowns needs to be relative and end in wildcard

torkelo · 2024-11-27T09:03:59Z

packages/scenes-app/src/components/Routes/Routes.tsx

+      <Route path={`${ROUTES.Demos}/*`} Component={DemoListPage} />
+      <Route path={`${ROUTES.GrafanaMonitoring}/*`} Component={GrafanaMonitoringApp} />
+      <Route path={`${ROUTES.ReactDemo}/*`} Component={ReactDemoPage} />
+      {/* <Redirect to={prefixRoute(ROUTES.Demos)} /> */}


Need to add a v6 redirect here

I think we can just make DemoListPage a fallback page instead?

<Route path="*" Component={DemoListPage} />

it's still missing, isn' it?

leventebalogh · 2024-12-16T13:10:24Z

I have spent sometime trying to green out all the test case, and although I think got somewhere, there are some weird things that might need someone with deeper knowledge of the scenes projects.

The main issue is with the tests under the scenes package: for some reason the inline jest.mock() calls don't seem to go through, while mocking via a __mocks__ directory is still working. Obviously it wouldn't make sense to transition all the tests to use a __mocks__ folder solution, we should instead find the root cause. (There was one issue we discovered with @jackw in the jest config: we also needed to introduce a "setupFiles" and move parts of the setupTestsAfterEnv.ts in there, although that didn't fix this problem.)

@grafana/dashboards-squad

oscarkilhed · 2024-12-18T09:32:26Z

I have spent sometime trying to green out all the test case, and although I think got somewhere, there are some weird things that might need someone with deeper knowledge of the scenes projects.

The main issue is with the tests under the scenes package: for some reason the inline jest.mock() calls don't seem to go through, while mocking via a __mocks__ directory is still working. Obviously it wouldn't make sense to transition all the tests to use a __mocks__ folder solution, we should instead find the root cause. (There was one issue we discovered with @jackw in the jest config: we also needed to introduce a "setupFiles" and move parts of the setupTestsAfterEnv.ts in there, although that didn't fix this problem.)

@grafana/dashboards-squad

@leventebalogh I see the tests are passing, did you find the issue?

oscarkilhed

Thanks for working on this @leventebalogh

How does this work for apps built with scenes that run in Grafana? Will they continue to work as long as they are built with an older version of scenes, or will they break when Grafana and scenes runs on router@v6?

leventebalogh · 2024-12-18T10:22:19Z

@leventebalogh I see the tests are passing, did you find the issue?

Yes, thanks @oscarkilhed, we managed to green them out with @jackw 👍

How does this work for apps built with scenes that run in Grafana? Will they continue to work as long as they are built with an older version of scenes, or will they break when Grafana and scenes runs on router@v6?

I am currently testing out these changes with an example plugin, and as soon as I am done with that I'll update the PR description with correct instructions on what steps plugins need to take for updating. (We will probably also have a migration page for this)

At the time being Grafana still supports plugins both using react-router@v5 or react-router@v6. This means, that plugins built with an earlier scenes version will still be supported (at least until core is updated to use react-router@v6 fully - then they will break). As far as I know scenes is bundled (and not externalised), so it shouldn't be a problem that core is depending on a different version (for now). I'll double check this though as well, and update the PR description!

This must be major version bump though, as any plugins updating to this scenes version will also need to update some things in their source code.

leventebalogh · 2024-12-20T08:54:03Z

@torkelo @dprokop could you please have a look if you think it's legit? (I have updated the PR description with a guide for upgrading plugins.)

Sergej-Vlasov · 2024-12-31T13:51:51Z

@leventebalogh do we need any additional steps for this to work in grafana/grafana? Did a draft scenes bump PR to launch a drone pipeline and quite a lot of tests are erroring out 🤔

leventebalogh · 2025-01-13T12:03:16Z

Thanks @Sergej-Vlasov, nice catch! 🙏

It looks like the issue was that core Grafana had a different version of react-router-dom than what was bundled with the @grafana/scenes package. To mitigate the issue I have moved the react-router-dom to be a peer and dev dependency of the scenes package. Hopefully the CI tests on your PR will get green, soon (I managed to green them out locally).

Sergej-Vlasov

@leventebalogh great job! This seems to be working now!

Since this is a major release lets postpone the merge a bit to allow any bugfixes/docs to land before hand. You can still start working on updating plugins in the mean time by using canary release of this PR.

We have the release notes with how to handle this breaking change but I think it would also be beneficial to include this in https://grafana.com/developers/scenes/

dprokop · 2025-01-14T10:25:18Z

packages/scenes-app/src/components/Routes/Routes.tsx

+      <Route path={`${ROUTES.Demos}/*`} Component={DemoListPage} />
+      <Route path={`${ROUTES.GrafanaMonitoring}/*`} Component={GrafanaMonitoringApp} />
+      <Route path={`${ROUTES.ReactDemo}/*`} Component={ReactDemoPage} />
+      {/* <Redirect to={prefixRoute(ROUTES.Demos)} /> */}


it's still missing, isn' it?

dprokop · 2025-01-14T11:53:06Z

packages/scenes-app/src/demos/variables.tsx

@@ -191,6 +194,7 @@ export function getVariablesDemo(defaults: SceneAppPageState) {
      new SceneAppPage({
        title: 'Many variable options',
        url: `${defaults.url}/many-values`,
+        routePath: 'many-values',


I'm wondering if we shouldn't make routePath mandatory, since it can be either a fixed string or parametrizable one. And all over the place in this PR the routePath is being added to the app pages. Don't think I saw a single app page in this PR that would not require it. WDYT?

Actually I think that's a good point, I couldn't think of any scenario where it wouldn't be necessary from now on (unless I am missing something). It could also help with migration I think for plugins (showing type errors for wherever it's missing). What do you think @torkelo?

@dprokop yea, I agree. Should be mandatory

leventebalogh · 2025-01-20T12:10:47Z

@Sergej-Vlasov Thanks, sure 👍

We have the release notes with how to handle this breaking change but I think it would also be beneficial to include this in https://grafana.com/developers/scenes/

Yeah, I think adding some migration docs would make sense as well, as this is a bit more "complicated" change, similar to what we have in https://grafana.com/developers/plugin-tools/migration-guides/update-from-grafana-versions/.

leventebalogh self-assigned this Nov 26, 2024

leventebalogh added the dependencies Update one or more dependencies version label Nov 26, 2024

leventebalogh requested review from torkelo, jackw, mdvictor and dprokop November 26, 2024 15:25

torkelo reviewed Nov 27, 2024

View reviewed changes

leventebalogh force-pushed the leventebalogh/update-react-router branch 4 times, most recently from e85e581 to 65f02c1 Compare December 16, 2024 13:03

leventebalogh force-pushed the leventebalogh/update-react-router branch from 77516f0 to 8ee5f00 Compare December 17, 2024 09:50

leventebalogh marked this pull request as ready for review December 17, 2024 12:19

leventebalogh added release Create a release when this pr is merged major Increment the major version when merged labels Dec 17, 2024

leventebalogh closed this Dec 17, 2024

leventebalogh reopened this Dec 17, 2024

oscarkilhed reviewed Dec 18, 2024

View reviewed changes

leventebalogh mentioned this pull request Dec 19, 2024

Scenes App: Update to use react-router v6 grafana/plugin-tools#1422

Draft

leventebalogh added 4 commits January 13, 2025 12:47

react-router: update to v6

ba787a7

fix: remove circular dependencies from tests

41fc361

fix: Use the new Router syntax for scenes-react tests

e57dabc

fix: change react-router-dom to be a dev and peer dependency

abedd6d

leventebalogh force-pushed the leventebalogh/update-react-router branch from c982524 to abedd6d Compare January 13, 2025 11:47

torkelo approved these changes Jan 13, 2025

View reviewed changes

Sergej-Vlasov approved these changes Jan 14, 2025

View reviewed changes

dprokop reviewed Jan 14, 2025

View reviewed changes

dprokop approved these changes Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to `react-router@6` #979

Update to `react-router@6` #979

leventebalogh commented Nov 26, 2024 •

edited by torkelo

Loading

torkelo commented Nov 27, 2024

torkelo Nov 27, 2024

leventebalogh Dec 13, 2024

dprokop Jan 14, 2025

leventebalogh commented Dec 16, 2024 •

edited

Loading

oscarkilhed commented Dec 18, 2024

oscarkilhed left a comment

leventebalogh commented Dec 18, 2024

leventebalogh commented Dec 20, 2024 •

edited

Loading

Sergej-Vlasov commented Dec 31, 2024

leventebalogh commented Jan 13, 2025

Sergej-Vlasov left a comment

dprokop Jan 14, 2025

dprokop Jan 14, 2025

leventebalogh Jan 20, 2025

torkelo Jan 20, 2025

leventebalogh commented Jan 20, 2025 •

edited

Loading

Update to react-router@6 #979

Are you sure you want to change the base?

Update to react-router@6 #979

Conversation

leventebalogh commented Nov 26, 2024 • edited by torkelo Loading

Why?

Release notes

Updating a plugin

Testing

torkelo commented Nov 27, 2024

torkelo Nov 27, 2024

Choose a reason for hiding this comment

leventebalogh Dec 13, 2024

Choose a reason for hiding this comment

dprokop Jan 14, 2025

Choose a reason for hiding this comment

leventebalogh commented Dec 16, 2024 • edited Loading

oscarkilhed commented Dec 18, 2024

oscarkilhed left a comment

Choose a reason for hiding this comment

leventebalogh commented Dec 18, 2024

leventebalogh commented Dec 20, 2024 • edited Loading

Sergej-Vlasov commented Dec 31, 2024

leventebalogh commented Jan 13, 2025

Sergej-Vlasov left a comment

Choose a reason for hiding this comment

dprokop Jan 14, 2025

Choose a reason for hiding this comment

dprokop Jan 14, 2025

Choose a reason for hiding this comment

leventebalogh Jan 20, 2025

Choose a reason for hiding this comment

torkelo Jan 20, 2025

Choose a reason for hiding this comment

leventebalogh commented Jan 20, 2025 • edited Loading

Update to `react-router@6` #979

Update to `react-router@6` #979

leventebalogh commented Nov 26, 2024 •

edited by torkelo

Loading

leventebalogh commented Dec 16, 2024 •

edited

Loading

leventebalogh commented Dec 20, 2024 •

edited

Loading

leventebalogh commented Jan 20, 2025 •

edited

Loading